Skip to content

Don't mark block as invalid when exception is unknown - #11231

Open
zilm13 wants to merge 4 commits into
Consensys-Incorporated:masterfrom
zilm13:block-import-exception
Open

zilm13 wants to merge 4 commits into
Consensys-Incorporated:masterfrom
zilm13:block-import-exception

Conversation

@zilm13

@zilm13 zilm13 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

PR Description

Replacement of broad rule which could mark block invalid incorrectly

Fixed Issue(s)

Fixes #11225

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

Note

Medium Risk
Changes consensus-adjacent fork-choice behavior when imports fail with internal errors; incorrect classification could still accept bad blocks or delay invalid detection, but it fixes wrongly blacklisting valid chain segments.

Overview
Tightens when INTERNAL_ERROR block imports are cached as invalid in BlockManager, so transient or node-local failures no longer poison the invalid-block cache.

Previously, any internal error except RejectedExecutionException caused dropInvalidBlock on INTERNAL_ERROR. The PR inverts that to an allowlist: only causes that clearly implicate block content—StateTransitionException, BlockProcessingException, SSZ/schema errors, BlsException, and ArithmeticException (including wrapped causes)—mark the block invalid. Errors like queue exhaustion, OOM, capacity limits, or unknown runtime failures leave the block unmarked, avoiding wrongful rejection of canonical chain descendants until cache eviction.

Tests are expanded to parameterized cases for both “do not mark invalid” and “mark invalid” exception families, with shared setup via setupBlockFailingImportWith.

Reviewed by Cursor Bugbot for commit 35bd3d8. Bugbot is set up for automated code reviews on this repo. Configure here.

}
return true;
private static boolean internalErrorToBeConsiderAsInvalidBlock(final Throwable internalError) {
// hasCause also checks the exception itself

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the original internalError instanceof RejectedExecutionException || ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's opposite originally

private boolean internalErrorToBeConsiderAsInvalidBlock(final Throwable internalError) {
  if (internalError instanceof RejectedExecutionException
      || ExceptionUtil.hasCause(internalError, RejectedExecutionException.class)) {
    return false;
  }

i guess because we don't want to stuck forever on EL malfunctioning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

INTERNAL_ERROR from local resource exhaustion permanently invalidates canonical blocks and their descendants

3 participants